-
-
Notifications
You must be signed in to change notification settings - Fork 351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Flood fill in java #1003
base: main
Are you sure you want to change the base?
Flood fill in java #1003
Conversation
Hi, thank you for your interest in the Algorithm Archive! However, the first step of admissibility is not yet reached: you need to get your code to compile on a Docker Ubuntu-based environment, which, as I am writing this, just failed.
Please fix it, so we can review and eventually merge your code in. |
Hi, |
Thank you for the reactivity (relative to timezones, of course)! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked your code before going to sleep, and it was empty.
Apparently your last commit truncated your flood_fill.java
file, and outright deleted your Main.java
file.
Please fix that, so we can review your code
Hi, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so while I'm not a Java specialist, the code was clear enough for me to approve it. I would ask for a second opinion, but I don't know anyone who is comfortable enough reviewing Java code :/
The only thing that annoys me is your package thing, but apart from that, I have nothing to say about the code.
if (p.x < 0 || p.x >= row || p.y < 0 || p.y >= col) return false; | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe you can just do:
if (p.x < 0 || p.x >= row || p.y < 0 || p.y >= col) return false; | |
return true; | |
return (p.x < 0 || p.x >= row || p.y < 0 || p.y >= col); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, wrong call: it was missing a !
operator before the parenthesis...
Co-authored-by: Sammy Plat <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, since you didn't respond to my last few comments, here is a new review.
Since you removed one line when removing the if
statement in inbound()
, you'll need to adjust the line numbers as well.
Looking forward to your next commit.
} | ||
public static boolean inbound(int[][] grid, Point p){ | ||
int row = grid.length, col = grid[0].length; | ||
return (p.x < 0 || p.x >= row || p.y < 0 || p.y >= col); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I mislead you earlier, this should be the correct way:
return (p.x < 0 || p.x >= row || p.y < 0 || p.y >= col); | |
return !(p.x < 0 || p.x >= row || p.y < 0 || p.y >= col); |
add java code for implementation of flood fill